Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quickstart guide for DHCP #68

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

mathewab
Copy link
Collaborator

@mathewab mathewab commented Feb 14, 2024

Added quickstart guide for DHCP
Made minor corrections to other documentation

}
````

!> Warning: Hard-coded credentials are not recommended in any configuration file. It is recommended to use environment variables.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that a valid markdown ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terraform Registry documentation accepts it as a special case. It shows up as a colored callout box.

https://developer.hashicorp.com/terraform/registry/providers/docs#callouts


````

We can now run `terraform plan` to see what resources will be created.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guides are usually written in an impersonal style. The beginning of the guid talks to the user with "You", it should continue to do so all along.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the "we" language.


Further, we will create a range and a fixed address reservation within the subnet.

We will use the following resources to create these
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"You"

dhcp_options = [
{
option_code = local.dhcp4_option_code_lookup["domain-name-servers"]
option_value = "1.1.1.1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid promoting a competitor (cloudflare's dns address)...
Use an address within the 10/16 network.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to 10.0.0.1

}
````

!> Warning: Hard-coded credentials are not recommended in any configuration file. It is recommended to use the AWS credentials file or environment variables.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!> markdown ?

@@ -24,5 +24,6 @@ testacc:

gen:
go generate
terraform-docs ./modules/bloxone_infra_host_aws
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
terraform-docs ./modules/bloxone_infra_host_aws
terraform-docs ./modules/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support future modules(GCP, Azure)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work. The terraform-docs requires the proper directory and does not recursively figure out the sub directories

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it, still doesn't work correctly. That works only for child modules. Our directory structure is a bit different.

````terraform
terraform {
required_providers {
bloxone = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should mention the terraform client version has to be >= 1.0.0.
Why do we want the provider version to 0.1.X? We are planning to release 1.0.0 and this wouldn't allow that. See: https://developer.hashicorp.com/terraform/tutorials/configuration-language/versions#review-example-configuration

Comment on lines 19 to 26
terraform {
required_providers {
bloxone = {
source = "infobloxopen/bloxone"
version = "~> 0.1.0"
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
terraform {
required_providers {
bloxone = {
source = "infobloxopen/bloxone"
version = "~> 0.1.0"
}
}
}
terraform {
required_providers {
bloxone = {
source = "infobloxopen/bloxone"
version = ">= 0.1.0"
}
}
required_version = ">= 1.0.0"
}

@venkat-iblox venkat-iblox merged commit fc8252f into infobloxopen:master Feb 14, 2024
3 checks passed
@mathewab mathewab deleted the docs-add-quickstart-guide branch February 15, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants